Skip to content

Remove CJS support. - #52

Open
davidlehn wants to merge 17 commits into
update-depsfrom
remove-cjs-support
Open

Remove CJS support.#52
davidlehn wants to merge 17 commits into
update-depsfrom
remove-cjs-support

Conversation

@davidlehn

Copy link
Copy Markdown
Member

No description provided.

@davidlehn
davidlehn requested review from applesnort and dlongley July 25, 2026 03:05
@codecov-commenter

codecov-commenter commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.54839% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.28%. Comparing base (2ad1aa2) to head (bb0518d).

Files with missing lines Patch % Lines
lib/httpClient.js 93.10% 2 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           update-deps      #52      +/-   ##
===============================================
- Coverage        94.05%   93.28%   -0.78%     
===============================================
  Files                4        3       -1     
  Lines              303      283      -20     
===============================================
- Hits               285      264      -21     
- Misses              18       19       +1     
Files with missing lines Coverage Δ
lib/index.js 100.00% <100.00%> (ø)
lib/httpClient.js 92.76% <93.10%> (-0.75%) ⬇️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ad1aa2...bb0518d. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dlongley dlongley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok from a glance -- hard to tell, without implementation experience, whether any new strange behavior is introduced.

davidlehn and others added 17 commits August 6, 2026 19:16
- Test on Node.js >=22.
- Update `engines.node` to `>=22`.
- Update README requirements section.
- Improve formatting.
- Add more common README sections.
- `kyOriginalPromise` no longer exported.
- `ky` is again exported.
- Change from using `ky` promises to regular instances.
- After `ky@2` update the error body is already available in
  `error.data` and trying to get JSON again will fail.
- Remove `push`.
- Add `query`, `options`, and `trace` to align with `ky@2`.
ky@2 merges header options via a plain object spread when both sides
are still plain objects, which does not dedupe names that differ only
by case (e.g. `Accept` vs `accept`), causing values to be appended
instead of overridden. Use a `Headers` instance instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Fix import.
- Use suggested ChromeHeadless options for CI.
Webpack's browser field remaps `tests/utils.js` to
`tests/utils-browser.js`, but the browser stub never defined
`makeAgent`. The namespace import in the shared spec file only
references it inside an `isNode` guard, but webpack still statically
validates the export, breaking the karma build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adding a non-simple header (e.g. `Authorization`) triggers a browser
CORS preflight `OPTIONS` request, which this route never answered,
causing the actual request to be blocked.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`ky@2` wraps the browser's `TypeError: Failed to fetch` in its own
`NetworkError`, with the original error moved to `cause`. Check both
locations so the friendly CORS message still gets applied.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@davidlehn
davidlehn force-pushed the remove-cjs-support branch from 80807f3 to bb0518d Compare August 6, 2026 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants